refactor: route fix through command module#165
refactor: route fix through command module#165ndycode wants to merge 3 commits intorefactor/pr1-fix-command-2from
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
lib/codex-manager.tsWhat Changed
runFix()wrappercodex auth fixdispatch to callrunFixCommand(...)directlyfixaction to use the same extracted command pathValidation
npm run test -- test/codex-manager-fix-command.test.ts test/codex-manager-cli.test.tsnpm run lintnpm run typechecknpm run buildRisk and Rollback
987fca2to restore the inline fix wrapperAdditional Notes
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
this pr completes the direct-routing cleanup for the fix command by replacing the deleted
runFix()wrapper with abuildFixCommandDeps()factory — directly addressing the duplicate-deps hazard flagged in the previous review. both the login-menu auto-fix path and the CLI dispatch now callrunFixCommandthrough the shared factory, keeping deps DRY. therunDoctor()wrapper is also removed and its deps are inlined at the single dispatch site.key changes:
runFix()wrapper deleted; replaced withbuildFixCommandDeps()factory used at both call sitesrunDoctor()wrapper deleted; doctor deps inlined directly atcommand === "doctor"dispatch (single call site, so no factory added)type FixCommandDepsimported to satisfy the factory's return type annotationissues found:
buildFixCommandDeps()has no dedicated vitest coverage; the fix-command test uses its own mock factory and the cli test mocks the full module, so the production wiring is exercised only in e2e — worth a smoke test given the 27-dep surface area and the 80% threshold requirementConfidence Score: 4/5
runFix()wrapper (typescript enforces completeness at compile time), both call sites are updated, and the cli integration test exercises the fix dispatch path. score is 4 rather than 5 becausebuildFixCommandDeps()is not directly unit-tested and the doctor inline creates a pattern asymmetry that could become a real bug if doctor gains a second call sitebuildFixCommandDepsfactory (no unit test) and the inlined doctor deps (no factory)Important Files Changed
runFix()andrunDoctor()wrappers; introducesbuildFixCommandDeps()factory to deduplicate the two fix call sites (login-menu + CLI dispatch); doctor command inlined without a factory — safe but asymmetric; no dedicated unit test for the new factory functionFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD CLI["runCodexMultiAuthCli(rawArgs)"] MENU["runAuthLogin → login-menu loop"] CLI -->|"command === 'fix'"| FIX_DISPATCH["buildFixCommandDeps()"] MENU -->|"menuResult.mode === 'fix'"| FIX_MENU["buildFixCommandDeps()"] FIX_DISPATCH --> RFC["runFixCommand(rest, deps)"] FIX_MENU --> RFC2["runFixCommand(['--live'], deps)"] CLI -->|"command === 'doctor'"| DOC_INLINE["inline DoctorCommandDeps object"] DOC_INLINE --> RDC["runDoctorCommand(rest, deps)"] style FIX_DISPATCH fill:#22c55e,color:#fff style FIX_MENU fill:#22c55e,color:#fff style DOC_INLINE fill:#f59e0b,color:#fffPrompt To Fix All With AI
Last reviewed commit: "refactor: share fix ..."